-
Notifications
You must be signed in to change notification settings - Fork 135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release/v2.0 #364
Merged
Merged
Release/v2.0 #364
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sangelovic
force-pushed
the
release/v2.0
branch
from
October 4, 2023 11:00
3571547
to
4d984d7
Compare
sangelovic
force-pushed
the
release/v2.0
branch
from
October 16, 2023 13:39
4d984d7
to
d1e7d8d
Compare
sangelovic
force-pushed
the
release/v2.0
branch
from
November 3, 2023 17:38
d8cee39
to
bb7047a
Compare
sangelovic
force-pushed
the
release/v2.0
branch
3 times, most recently
from
December 5, 2023 18:09
0acfe8d
to
25f812c
Compare
sangelovic
force-pushed
the
release/v2.0
branch
from
December 30, 2023 20:12
1bd96c1
to
f7afe3d
Compare
sangelovic
force-pushed
the
release/v2.0
branch
from
February 18, 2024 17:23
fbb4a23
to
6f8f936
Compare
This was referenced Feb 20, 2024
sangelovic
force-pushed
the
release/v2.0
branch
2 times, most recently
from
April 1, 2024 12:04
4ee885b
to
0aa32c5
Compare
sangelovic
force-pushed
the
release/v2.0
branch
3 times, most recently
from
April 16, 2024 20:51
de6a209
to
287c52a
Compare
sangelovic
force-pushed
the
release/v2.0
branch
from
April 24, 2024 08:17
eb6250c
to
f069212
Compare
See #324 for discussion
This will also install the library as `libsdbus-c++.so.2.0.0`, which represents the actual version of this library
Signatures of callbacks async_reply_handler, signal_handler, message_handler and property_set_callback were modified to take input message objects by value, as opposed to non-const ref. The callee assumes ownership of the message. This API is more idiomatic, more expressive, cleaner and safer. Move semantics is used to pass messages to the callback handlers. In some cases, this also improves performance.
Since synchronous D-Bus calls are simplified in v2.0 and no more implemented in terms of an async call, the issue reported in #362 is no more relevant, and we can return to the simple boolean flag for indicating a finished call.
The test now works with Clang, libc++ and -O2 optimization, since the underlying implementation has been completely re-designed and doesn't suffer from that problem anymore.
This makes the library more robust and prone to user's errors when the user writes an extension for their custom type. In case they forget to implement a serialization function for that type and yet insert an object of that type into sdbus::Message, the current behavior is that, surprisingly, the library masks the error as it resolves the call to the Variant overload, because Variant provides an implicit template converting constructor, so the library tries to construct first the Variant object from the object of custom type, and then inserting into the message that Variant object. Variant constructor serializes the underlying object into its internal message object, which resolves to the same message insertion overload, creating an infinite recursion and ultimately the stack overflow. This is undesired and plain wrong. Marking this Variant converting constructor solves these problems, plus in overall it makes the code a little safer and more verbose. With explicit Variant constructor, when the user forgets to implement a serialization function for their type, the call of such function will fail with an expressive compilation error, and will produce no undesired, surprising results.
In sdbus-c++ v1, new virtual functions (e.g. overloads of existing virtual functions) were always placed at the end of the class to keep backwards ABI compatibility. Now, with v2, these functions are reordered and functions forming a logical group are together.
This PR makes things around connection factories a little more consistent and more intuitive: * createConnection() has been removed. One shall call more expressive createSystemConnection() instead to get a connection to the system bus. * createDefaultBusConnection() has been renamed to createBusConnection(), so as not to be confused with libsystemd's default_bus, which is a different thing (a reusable thread-local bus). Proxies still by default call createBusConnection() to get a connection when the connection is not provided explicitly by the caller, but now createBusConnection() does a different thing, so now the proxies connect to either session bus or system bus depending on the context (as opposed to always to system bus like before). The integration tests were modified to use createBusConnection().
A new CMake configuration variable SDBUSCPP_SDBUS_LIB has been introduced that enables clients to specify which sd-bus implementation library shall be used by sdbus-c++.
As of now sdbus-c++ supports C++20 but does not require it, and the used C++20 features are conditionally compiled depending on whether they are available or not.
This introduces strong types for `std::string`-based D-Bus types. This facilitates safer, less error-prone and more expressive API. What previously was `auto proxy = createProxy("org.sdbuscpp.concatenator", "/org/sdbuscpp/concatenator");` is now written like `auto proxy = createProxy(ServiceName{"org.sdbuscpp.concatenator"}, ObjectPath{"/org/sdbuscpp/concatenator"});`. These types are: * `ObjectPath` type for the object path (the type has been around already but now is also used consistently in sdbus-c++ API for object path strings), * `InterfaceName` type for D-Bus interface names, * `BusName` (and its aliases `ServiceName` and `ConnectionName`) type for bus/service/connection names, * `MemberName` (and its aliases `MethodName`, `SignalName` and `PropertyName`) type for D-Bus method, signal and property names, * `Signature` type for the D-Bus signature (the type has been around already but now is also used consistently in sdbus-c++ API for signature strings), * `Error::Name` type for D-Bus error names.
Having explicit conversion operator is a good practice according to the C++ core guidelines, as it makes the code safer and better follows the principle of least astonishment. Also, it is consistent with the standard library style, where wrappers like std::variant, std::any, std::optional... also do not provide an implicit conversion to the underlying type. Last but not least, it paves the way for the upcoming std::variant <-> sdbus::Variant implicit conversions without surprising behavior in some edge cases.
Since sdbus-c++ knows types of D-Bus call/signal/property input/output parameters at compile time, why not assemble the D-Bus signature strings at compile time, too, instead of assembling them at run time as std::string with likely cost of (unnecessary) heap allocations... std::array is well supported constexpr type in C++20, so we harness it to build the D-Bus signature string and store it into the binary at compile time.
Moving adaptor or proxy instances changes their `this` pointer. But `this` is captured by value in closures used by those instances, and this remains unchanged on move, leading to accessing an invalid instance when a lambda expression executes. Supporting move semantics would require unsubscribing/unregistering vtable, handlers, etc. and re-subscribing and re-registering all that, which is too complicated and may have side effects. Hence it has been decided that these classes are not moveable. One may use an indirection with e.g. `std::unique_ptr` to get move semantics.
This makes the signature of addObjectManager() function overloads consistent with common API design of the library.
sangelovic
force-pushed
the
release/v2.0
branch
from
April 24, 2024 18:16
85ee3f5
to
6eb0cff
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an ongoing, WIP, draft MR. Once all the work on v2 API is finished, the master branch will get branched off to the
release/v1.x
branch and therelease/v2.0
branch will get merged tomaster
. v2 will become the default development version.